fix(#150): add maxlength on input #222
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #222 +/- ##
======================================
Coverage 6.42% 6.42%
======================================
Files 11 11
Lines 1712 1712
======================================
Hits 110 110
Misses 1594 1594
Partials 8 8 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thanks for your contribution @jufab 🎉
Mostly looks good, I have just one request to use type inference to avoid the use of @ts-ignore plus a few small comments. Otherwise LGTM 👍
| @@ -16,6 +16,7 @@ import {FormatOptionLabelContext} from 'react-select/src/Select'; | |||
| import {getColorStyles, getDescription, getProfilePicture} from '../../utils'; | |||
|
|
|||
| import './autocomplete_selector.scss'; | |||
| import {components} from 'react-select'; | |||
There was a problem hiding this comment.
This import should be further up with the other react-select imports. This should really be caught by the linter but I think that rule is not set up for this project
| @@ -30,6 +31,7 @@ type Props = { | |||
| theme: Theme, | |||
| } | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Nit: Unnecessary extra line here
| //@ts-ignore | ||
| const Input = (props) => <components.Input {...props} maxLength={22} />; |
There was a problem hiding this comment.
Ideally we don't need to use @ts-ignore. I think we can inline the declaration of this below when defining the components prop, and typescript will automatically infer the type of props
components={{
Input: (props) => <components.Input {...props} maxLength={22} />,
}}There was a problem hiding this comment.
If I inline this code, I have an other problem. 😢
There was a problem hiding this comment.
Okay then it seems the types for the library don't agree with its supported functionality. Yeah let's just use @ts-ignore here. Thanks for trying this 👍
|
Also there are a few linting issues being reported by CI: Can you please take a look at these errors @jufab? Thank you! |
There was a problem hiding this comment.
Tested this PR for the following scenarios:-
- Username field can accept maximum length upto 22 characters while creating a ToDo.
- Username field can accept maximum length upto 22 characters while assigning a ToDo.
The PR was found working fine for both the above conditions, LGTM. Approved.
mickmister
added
4: Reviews Complete
|
Thanks @jufab!! |
Summary
Add a Max Length on user input.
Max length = 22
Ticket Link
Fixe #150